fix: prevent invalid uniqueId of -1 from being added to bubblePath in…#2493
Conversation
🦋 Changeset detectedLatest commit: b361331 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR introduces a patch-level fix for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web-platform/web-core/tests/element-apis.spec.ts (1)
71-79: Assert the valid ancestor remains inbubblePath.This test would still pass if
#commonEventHandlerreturned an empty path. Please also assert thatnode1’s valid unique ID is preserved.Proposed test tightening
expect(mtsBinding.wasmContext!.common_event_handler).toHaveBeenCalled(); const calls = (mtsBinding.wasmContext!.common_event_handler as any).mock.calls; - const bubblePath = calls[0][1] as Uint32Array; + const bubblePath = Array.from(calls[0][1] as Uint32Array); + const invalidUniqueIdInUint32Array = 0xffffffff; - // Check that -1 (4294967295 in Uint32Array) is NOT in the bubblePath - for (let i = 0; i < bubblePath.length; i++) { - expect(bubblePath[i] !== 4294967295).toBe(true); - } + expect(bubblePath).toEqual([mtsGlobalThis.__GetElementUniqueID(node1)]); + expect(bubblePath).not.toContain(invalidUniqueIdInUint32Array);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web-platform/web-core/tests/element-apis.spec.ts` around lines 71 - 79, The test currently only asserts that -1 isn't present in the bubblePath returned from mtsBinding.wasmContext!.common_event_handler but would still pass if the path were empty; update the assertion to also verify that node1's valid unique ID is included in the bubblePath. After retrieving bubblePath (from (mtsBinding.wasmContext!.common_event_handler as any).mock.calls[0][1]), compute or read node1's unique ID (the same identifier used when creating node1) and assert that bubblePath contains that ID (e.g., convert bubblePath to a normal array and expect it to contain node1's ID) so the test ensures the ancestor is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/web-platform/web-core/tests/element-apis.spec.ts`:
- Around line 71-79: The test currently only asserts that -1 isn't present in
the bubblePath returned from mtsBinding.wasmContext!.common_event_handler but
would still pass if the path were empty; update the assertion to also verify
that node1's valid unique ID is included in the bubblePath. After retrieving
bubblePath (from (mtsBinding.wasmContext!.common_event_handler as
any).mock.calls[0][1]), compute or read node1's unique ID (the same identifier
used when creating node1) and assert that bubblePath contains that ID (e.g.,
convert bubblePath to a normal array and expect it to contain node1's ID) so the
test ensures the ancestor is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48258bf1-107a-4d4e-ad95-5046f713296a
📒 Files selected for processing (3)
.changeset/fix-event-handler-unique-id.mdpackages/web-platform/web-core/tests/element-apis.spec.tspackages/web-platform/web-core/ts/client/mainthread/elementAPIs/WASMJSBinding.ts
Merging this PR will degrade performance by 16.16%
Performance Changes
Comparing Footnotes
|
React External#574 Bundle Size — 583.27KiB (0%).b361331(current) vs 02be891 main#571(baseline) Bundle metrics
|
| Current #574 |
Baseline #571 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
17 |
17 |
|
5 |
5 |
|
8.59% |
8.59% |
|
0 |
0 |
|
0 |
0 |
Bundle analysis report Branch PupilTong:p/hw/fix-event-with-in... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#9031 Bundle Size — 898.16KiB (~+0.01%).b361331(current) vs 02be891 main#9028(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch PupilTong:p/hw/fix-event-with-in... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#7456 Bundle Size — 224.41KiB (0%).b361331(current) vs 02be891 main#7453(baseline) Bundle metrics
|
| Current #7456 |
Baseline #7453 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
179 |
179 |
|
69 |
69 |
|
44.5% |
44.5% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #7456 |
Baseline #7453 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
78.65KiB |
78.65KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-event-with-in... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#589 Bundle Size — 195.57KiB (0%).b361331(current) vs 02be891 main#586(baseline) Bundle metrics
|
| Current #589 |
Baseline #586 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
3 |
3 |
|
173 |
173 |
|
66 |
66 |
|
43.99% |
43.99% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #589 |
Baseline #586 |
|
|---|---|---|
111.23KiB |
111.23KiB |
|
84.34KiB |
84.34KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-event-with-in... Project dashboard
Generated by RelativeCI Documentation Report issue
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/react@0.120.0 ### Minor Changes - Bump `@lynx-js/internal-preact` from `10.28.4-dfff9aa` to `10.29.1-20260424024911-12b794f` ([diff](lynx-family/internal-preact@10.28.4-dfff9aa...10.29.1-20260424024911-12b794f)). ([#2512](#2512)) Fixes wrong DOM order when a keyed child moves to a different `$N` slot across a re-render. Cross-slot moves now land at the correct slot position instead of being appended past stable siblings. - Refactor preact to support multi-slots children and reduce the number and depth of snapshot. ([#1764](#1764)) ### Patch Changes - Fix stale callback-local references when transforming JSX inside `children={array.map(...)}` prop expressions. ([#2524](#2524)) - Fix ref callbacks not being cleaned up or re-applied correctly when the ref at the same element slot changes across rerenders that happen before hydration (e.g. a `useEffect` triggering `setState` during the initial background render). ([#2500](#2500)) - fix: reduce redundant updates for main-thread handlers and gestures ([#2188](#2188)) - Updates are faster when the main-thread event handler or gesture object is stable across rerenders (fewer unnecessary native updates). - Spread props rerenders that don't semantically change the handler/gesture no longer trigger redundant updates. - Removing a gesture from spread props reliably clears the gesture state on the target element. - Fix hydration edge cases by tolerating serialized snapshot nodes with missing `values` ([#2481](#2481)) - Keep ReactLynx Testing Library imports aligned with the contained snapshot runtime paths. ([#2498](#2498)) ## @lynx-js/template-webpack-plugin@0.11.0 ### Minor Changes - Add CSS source map support and source-mapped template encode diagnostics. ([#2483](#2483)) ### Patch Changes - fix: genStyleInfo should also preserve CSS variable fallback values when encoding web-core stylesheets so declarations like `var(--token, rgba(...))` are emitted with their fallback intact. ([#2502](#2502)) - Updated dependencies \[[`e179680`](e179680), [`8352530`](8352530), [`30f0277`](30f0277), [`887b8aa`](887b8aa), [`1d4abfc`](1d4abfc), [`25e196b`](25e196b), [`fb7bc84`](fb7bc84), [`9e149c4`](9e149c4), [`30f0277`](30f0277), [`9e149c4`](9e149c4)]: - @lynx-js/css-serializer@0.1.6 - @lynx-js/web-core@0.20.3 ## @lynx-js/i18next-translation-dedupe@0.0.1 ### Patch Changes - Introduce `@lynx-js/i18next-translation-dedupe` package to avoid bundling i18next translations twice in Lynx apps. ([#2482](#2482)) The package reads translations extracted by `rsbuild-plugin-i18next-extractor`, skips the extractor's default rendered asset, and writes the translations into the Lynx bundle custom section: ```json { "customSections": { "i18next-translations": { "content": { "en-US": { "hello": "Hello" }, "zh-CN": { "hello": "你好" } } } } } ``` ## @lynx-js/docs-mcp-server@0.2.2 ### Patch Changes - Fix Windows startup error. ([#2474](#2474)) ## @lynx-js/react-umd@0.120.0 ### Patch Changes - Support compile main-thread script to bytecode in external bundle ([#2459](#2459)) ## @lynx-js/rspeedy@0.14.3 ### Patch Changes - add a `sourceMap.css` option to emit CSS sourcemaps. ([#2442](#2442)) By default, `sourceMap.css` is false. You can set it to true to emit CSS sourcemaps. ```js import { defineConfig } from "@lynx-js/rspeedy"; export default defineConfig({ output: { sourceMap: { css: true, }, }, }); ``` - bump rsdoctor to 1.5.6 ([#2410](#2410)) - Enable CSS source maps by default in Rspeedy output config. ([#2483](#2483)) - Prefer physical routable IPv4 addresses over tunnel and link-local interfaces when resolving the dev host IP for generated preview and bundle URLs. ([#2409](#2409)) - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.20.3 ## @lynx-js/lynx-bundle-rslib-config@0.3.2 ### Patch Changes - Support compile main-thread script to bytecode in external bundle ([#2459](#2459)) - Updated dependencies \[[`e179680`](e179680)]: - @lynx-js/css-serializer@0.1.6 ## @lynx-js/react-rsbuild-plugin@0.16.1 ### Patch Changes - Respect `dev.hmr: false` when installing React Refresh integrations so disabled HMR no longer injects the refresh loader or plugin. ([#2487](#2487)) - Fix stale callback-local references when transforming JSX inside `children={array.map(...)}` prop expressions. ([#2524](#2524)) - Supports @lynx-js/react 0.120.0 ([#1764](#1764)) - Updated dependencies \[[`e179680`](e179680), [`13655ac`](13655ac), [`f15494b`](f15494b), [`e179680`](e179680), [`e179680`](e179680)]: - @lynx-js/template-webpack-plugin@0.11.0 - @lynx-js/css-extract-webpack-plugin@0.7.1 - @lynx-js/react-webpack-plugin@0.9.2 - @lynx-js/react-alias-rsbuild-plugin@0.16.1 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.5 ## @lynx-js/css-serializer@0.1.6 ### Patch Changes - Add CSS source map support and source-mapped template encode diagnostics. ([#2483](#2483)) ## @lynx-js/web-core@0.20.3 ### Patch Changes - fix: `__AddClass` triggers style updates when `enableCSSSelector` is `false` ([#2515](#2515)) `__AddClass` was missing the expected call to `update_css_og_style` when CSS selectors are disabled (`enableCSSSelector: false`). With this fix, dynamically adding a class correctly delegates style population from the template AST into the DOM, mirroring the behavior of `__SetClasses`. Added behavioral unit test and end-to-end playwright validations using dynamically generated JSON AST `styleInfo` mocks. - fix(web-core): skip setting lynxEntryNameAttribute for **Card** and use constants for server element APIs ([#2510](#2510)) - Fix componentCSSID behavior for SSR and main thread by calculating element css_id from parent component correctly. ([#2495](#2495)) - fix: avoid panic in dispatch_event_by_path when element data cannot be retrieved ([#2508](#2508)) - fix: filter out -1 uniqueId in commonEventHandler ([#2493](#2493)) - feat: add x-markdown support ([#2412](#2412)) Add opt-in support for the `x-markdown` element on Lynx Web, including Markdown rendering together with its related styling, interaction, animation, truncation, range rendering, and effect capabilities exposed through the component API. Update the `web-core`, `web-core-wasm`, and `web-mainthread-apis` runtime paths to use the shared property-or-attribute setter from `web-constants`, so custom elements such as `x-markdown` can receive structured property values correctly instead of being forced through string-only attribute updates. ```javascript import "@lynx-js/web-elements/XMarkdown"; ``` - fix: transformVH not work with cqw unit as the base length ([#2469](#2469)) - fix: add cardType resolution for legacy json lynx bundle ([#2510](#2510)) - fix: the default value of rpx is supposed to be 1/750 cqw ([#2469](#2469)) - Updated dependencies \[[`e179680`](e179680), [`647334c`](647334c), [`fb7bc84`](fb7bc84), [`9454dc4`](9454dc4), [`bdec498`](bdec498), [`b0247f9`](b0247f9), [`eec539a`](eec539a)]: - @lynx-js/css-serializer@0.1.6 - @lynx-js/web-elements@0.12.1 - @lynx-js/web-worker-rpc@0.20.3 ## @lynx-js/web-elements@0.12.1 ### Patch Changes - fix: XMarkdown slot created should not have prefix ([#2520](#2520)) - feat: add x-markdown support ([#2412](#2412)) Add opt-in support for the `x-markdown` element on Lynx Web, including Markdown rendering together with its related styling, interaction, animation, truncation, range rendering, and effect capabilities exposed through the component API. Update the `web-core`, `web-core-wasm`, and `web-mainthread-apis` runtime paths to use the shared property-or-attribute setter from `web-constants`, so custom elements such as `x-markdown` can receive structured property values correctly instead of being forced through string-only attribute updates. ```javascript import "@lynx-js/web-elements/XMarkdown"; ``` - fix: x-markdown inline view injection no longer queries light DOM children when the content attribute changes. Consumers must now pre-set `slot="{id}"` on the child element they want to project into `inlineview://{id}`. ([#2516](#2516)) - fix: list cannot drag-scroll inside x-foldview-slot-ng ([#2507](#2507)) Cause: `touchstart` used `elementsFromPoint(pageX, pageY)` (expects `clientX/clientY`), so hit-testing can miss the real inner scroller (e.g. `x-list` shadow `#content`) when the document is scrolled. Fix: use `elementsFromPoint(clientX, clientY)` + `event.composedPath()` for Shadow DOM, and keep `previousPageX` updated during `touchmove`. - fix: line-height of markdown-style should be added `px` ([#2509](#2509)) - fix: list `bindscrolltolower` may not trigger because the lower threshold ([#2484](#2484)) sentinel had no effective size or offset, causing the bottom `IntersectionObserver` to miss the list boundary ## @lynx-js/web-explorer@0.0.17 ### Patch Changes - bump rsdoctor to 1.5.6 ([#2410](#2410)) ## @lynx-js/css-extract-webpack-plugin@0.7.1 ### Patch Changes - Fix CSS source map line offsets when wrapping extracted CSS with cssId metadata. ([#2514](#2514)) - Support `@lynx-js/template-webpack-plugin` v0.11.0. ([#2483](#2483)) ## @lynx-js/react-webpack-plugin@0.9.2 ### Patch Changes - Support `@lynx-js/template-webpack-plugin` v0.11.0. ([#2483](#2483)) ## create-rspeedy@0.14.3 ## @lynx-js/react-alias-rsbuild-plugin@0.16.1 ## upgrade-rspeedy@0.14.3 ## @lynx-js/web-rsbuild-server-middleware@0.20.3 ## @lynx-js/web-worker-rpc@0.20.3 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… WASMJSBinding
Summary by CodeRabbit
Checklist